Skip to content

ci: stop rebuilding what has not changed - #1150

Open
kvinwang wants to merge 4 commits into
nextfrom
ci/speed-up-pr-checks
Open

ci: stop rebuilding what has not changed#1150
kvinwang wants to merge 4 commits into
nextfrom
ci/speed-up-pr-checks

Conversation

@kvinwang

@kvinwang kvinwang commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

A PR push currently costs about 120 runner-minutes. Most of it is the same
Rust workspace being compiled again, and jobs running on changes they cannot be
affected by.

Measured from the last 12 runs of each workflow, and from one push on #1147:

job workflow
17.4m Analyze (rust) CodeQL default setup — see #1151
11.7m rust-checks rust.yml
8.3m sdk-tests sdk.yaml
8.0m / 7.5m / 7.0m kms / gateway / verifier docker-build-check.yml
7.6m + 5.6m the two compat legs sdk-compat.yaml

What this changes

Caching. rust-checks is the largest job that gates every merge and it had
no cargo cache at all. Neither did sdk-tests. simulator-release.yml has been
using Swatinem/rust-cache all along, so this is the repository's own pattern
applied to the jobs that need it most.

Filters. rust.yml, sdk.yaml, sdk-compat.yaml, docker-build-check.yml
and vmm-ui.yml had no paths: at all: a one-line documentation change ran
roughly 50 minutes of Rust compilation.

rust-checks and sdk-tests are required status checks, so they cannot
take a workflow-level paths: — a skipped workflow reports nothing, the check
never arrives, and the pull request waits on it forever. Those two get the
filter as a step inside a job that always runs and always reports; the
expensive steps are gated on its output. The three that are not required take
an ordinary paths:.

One agent build instead of two. Both sdk-compat legs built
dstack-guest-agent-simulator from the same commit, in parallel. A simulator
job now populates the cargo cache first and both legs restore it. They still
run the build themselves, so what they test is still what the commit produces.

Rust CodeQL — the largest job of all — is not here. Moving it needs a
repository setting changed in the same breath, so it is #1151.

What was checked

actionlint and prek run --all-files are clean.

The two gate patterns were tested against a file list rather than reasoned
about:

path rust-checks sdk-tests
docs/deployment.md skip skip
README.md skip skip
os/mkosi/… skip skip
dstack/gateway/src/main.rs run run
dstack/ra-tls/src/lib.rs run run
sdk/python/… skip run
sdk/simulator/build.sh run run
rust-toolchain.toml run run

A push with no base SHA yields relevant=true, so a non-PR build never skips.

That table is also where a real bug surfaced. The sdk-tests pattern started as
sdk/ only, which was wrong: sdk/run-tests.sh calls simulator_start, and
the simulator is built from the dstack workspace — these suites exercise the
agent's wire surface, not just the client libraries, so a dstack/ change must
run them. Both sdk.yaml and sdk-compat.yaml now name the whole directory
rather than the simulator's dependency closure: the closure is a dozen crates
deep and would go stale the first time one of them moved, and a filter that
quietly stops covering something is worse than one that occasionally runs when
it need not.

Not done here

  • The gateway suites' fixture images build with no exported layer cache, so on a
    fresh runner they compile Rust from cold every time. Fixing that means
    cache_to: type=gha on the compose builds; it belongs with those suites
    rather than with the shared CI configuration.
  • rust-checks compiles the workspace twice — cargo clippy with default
    features, then cargo test --all-features — because the two feature
    resolutions share few artifacts. Aligning them would also make clippy lint the
    test code, which it currently does not (--all-targets is missing). Worth
    measuring first, since it may surface new lints.

`rust-checks` is the largest job in the repository at ~12 minutes, it is a
required status check, and it was compiling the workspace from an empty target
directory on every run. Every other Rust job in the repository is in the same
state except two: the gateway suites, and `simulator-release.yml`, which has
been using `Swatinem/rust-cache` all along. This uses the same action the same
way.

`workspaces: dstack` because the manifest is not at the repository root, and
the action keys the cache on the lockfile it finds there.
…moved

Two changes to the same job, both about work it did not need to do.

The cache is the same one `rust-checks` just got; `sdk/rust` is a separate
workspace from `dstack` and gets its own entry.

The filter is a step, not a `paths:` on the workflow. `sdk-tests` is a required
status check: a workflow skipped by `paths:` reports nothing at all, so the
check never arrives and the pull request waits on it forever. Gating the
expensive steps inside a job that always runs keeps the report and drops the
work -- a documentation-only pull request costs a checkout and a diff.

The pattern includes `dstack/` because `sdk/run-tests.sh` starts the simulator,
which is built from that workspace: these suites exercise the agent's wire
surface, not only the client libraries. The whole directory rather than the
simulator's dependency closure, which is a dozen crates deep and would go stale
the first time one of them moved.
Neither is a required status check, so a workflow-level `paths:` filter is safe
here -- a pull request that skips them is not left waiting on a check that
never arrives.

Docker Build Check verifies that the three builder images still build and that
their pinned package lists still match, which cannot change unless something
under `dstack/` does. It is three jobs of roughly eight minutes each and it was
running on every pull request, including documentation-only ones.

The images also get a layer cache, scoped per image. It covers the
pinned-package install and the toolchain setup and stops there: the cargo build
cannot be cached, because the source arrives through `git clone` at DSTACK_REV
inside the build rather than from the build context, so BuildKit has nothing to
key it on. That is deliberate -- the revision is what the image records in
/etc/.GIT_REV -- so the ceiling is the layers above the clone.

The VMM UI build is a minute and only ever concerns `dstack/vmm/ui`.
Both matrix legs call `simulator_start`, which builds
`dstack-guest-agent-simulator` out of the current tree. Same binary, same
commit, compiled twice in parallel.

The legs still build it themselves -- nothing here replaces that call or hands
them a binary from elsewhere, so what they test is still what the commit
produces. A `simulator` job runs first and populates the cargo cache under a key
both legs restore, so the build they run finds its work already done. A skip
switch in `simulator_build` would have been shorter and would have created a
path where the binary under test did not come from the checkout, which is not
worth a few minutes.

The workflow also gains a `paths:` filter. `dstack/**` rather than the
guest-agent directories alone: the closure reaches a dozen crates, and naming
the obvious three would leave the rest silently uncovered.
Copilot AI lite review requested due to automatic review settings August 27, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants